Solution Pool #3657
Solution Pool #3657
Conversation
Reordering and documenting API
Use the Pyomo Bunch class as an alias for Munch, to avoid introducing an additional Pyomo dependency.
|
Note that this PR now includes updates to the Bunch class defined in pyomo.common. I have been using the public Munch class, but these revisions align the Bunch API with Munch. |
Avoiding use of KW_ONLY, which is an internal mechanism
Using new serialization API, which is simpler. :)
1. Reworking solver matrix logic 2. Fixing test to benchmark against the solution values
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3657 +/- ##
==========================================
- Coverage 89.31% 87.20% -2.12%
==========================================
Files 896 826 -70
Lines 103682 102378 -1304
==========================================
- Hits 92605 89280 -3325
- Misses 11077 13098 +2021
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Added non-positive error check and value 1 warning for num_solutions in balas
Added num_solution error if num_solutions is non-positive, warning if num_solutions =1
Added more test for pass through parameters and applied black to the files
Solnpool Updates to enforce clarity on what metadata versus pool_config objects describe
1. Using keyword names when adding solutions pools to a pool manager 2. Many documentation changes 3. Renamed Variable and Objective classes to avoid conflicts with Pyomo naming (in sphinx) 4. Clarifyied the semantics of the gurobi enumeration methods.
Includes documentation with solution pools
|
@emma58 @michaelbynum FYI, this PR is ready for review. |
|
@whart222 - Can you please resolve the merge conflicts? |
|
@mrmundt @emma58 @michaelbynum Merge conflicts have been resolved. |
|
@emma58 @michaelbynum FYI, a CMU collaborator recently highlighted that the approval of this PR is blocking CMU software commits to snoglode that would leverage this capability. |
mrmundt
left a comment
mrmundt
left a comment
There was a problem hiding this comment.
This is by no means comprehensive, but I wanted to provide at least some feedback to start.
| def test_non_positive_num_solutions(self, mip_solver): | ||
| """ | ||
| Confirm that an exception is thrown with a non-positive num solutions | ||
| """ | ||
| m = tc.get_triangle_ip() | ||
| try: | ||
| enumerate_binary_solutions(m, num_solutions=-1, solver=mip_solver) | ||
| except AssertionError as e: | ||
| pass |
There was a problem hiding this comment.
This test doesn't really make sense to me. If you want to actually check that an exception is thrown, wouldn't it be better to use something like:
with self.assertRaises(WhateverException):
--- do the thing ---
| try: | ||
| gurobi_enumerate_linear_solutions(n, num_solutions=-1) | ||
| except AssertionError as e: | ||
| pass |
| try: | ||
| gurobi_generate_solutions(m, num_solutions=-1) | ||
| except AssertionError as e: | ||
| pass | ||
|
|
||
| def test_search_mode(self): | ||
| """ | ||
| Confirm that an exception is thrown with pool_search_mode not in [1,2] | ||
| """ | ||
| m = tc.get_triangle_ip() | ||
| try: | ||
| gurobi_generate_solutions(m, pool_search_mode=0) | ||
| except AssertionError as e: | ||
| pass |
| try: | ||
| lp_enum.enumerate_linear_solutions(m, num_solutions=-1, solver=mip_solver) | ||
| except AssertionError as e: | ||
| pass |
emma58
left a comment
emma58
left a comment
There was a problem hiding this comment.
Sorry for the huuuuge number of comments, but there's a lot here! Many are minor, but there are a few design questions too: The biggest one is that I'm wondering if the VariableInfo and ObjectiveInfo classes could store a reference to the original pyomo component rather than copying a lot but not all of the same data. It seems like that would make it much easier to map solutions in the pool to the original model, which I imagine will be a common user need.
| * alternative optimal solutions can be used to assess trade-offs between | ||
| competing objectives; | ||
| * comparisons amongst alternative solutions provide | ||
| insights into the efficacy of model predictions with | ||
| inaccurate or untrusted optimization formulations; or | ||
| * alternative solutions can be identified to support the future analysis of model revisions (e.g. to | ||
| account for previously unexpressed constraints). |
There was a problem hiding this comment.
It seems like there is also just the obvious use case of wanting to enable a decision maker to explore the optimal space: for example, for a design optimization.
| be configured with solver names and options, and they return a pool of | ||
| solutions for the pyomo model. However, these functions are independent | ||
| of pyomo's solver interface because they return a custom solution object. | ||
| of pyomo's solver interface because they return a custom pool manager object. |
There was a problem hiding this comment.
typo: interface -> interfaces
| * ``enumerate_linear_solutions`` | ||
| * :py:func:`enumerate_linear_solutions` | ||
|
|
||
| * Finds alternative optimal solutions for a (mixed-integer) linear program. |
There was a problem hiding this comment.
This is the only one that doesn't say how it generates alternative optimal solutions.
| """ | ||
| # TODO: this does not set an active pool, should we do that? | ||
| # TODO: this does not seem to update the counter value, possibly leading to non-unique ids |
There was a problem hiding this comment.
Is this still true? Does that break any of your assumptions in this class?
There was a problem hiding this comment.
Yes, this is a problem. I need to think about this. :(
| class PyomoPoolManager(PoolManager): | ||
| """ | ||
| A subclass of PoolManager for handing pools of Pyomo solutions. | ||
| This class redefines the add_pool method to use the _as_pyomo_solution method to construct Solution objects. | ||
| Otherwise, this class inherits from PoolManager. | ||
| """ |
There was a problem hiding this comment.
Why not make the converstion-to-solution method an argument to the PoolManager constructor, and then you don't need this class at all--that can just be changed to _as_pyomo_solution to get this functionality.
There was a problem hiding this comment.
This is an interesting question. Would we ever want to have a pool manager manage pools with different as_solution functions? Right now, that's supported. But I'm not sure whether that makes sense. I'll discuss this with the devs.
There was a problem hiding this comment.
There still seems a motivation for PyomoPoolManager, to avoid exposing the _as_pyomo_solution function. This is a matter of design, though. I think it's simpler for an end-user to use a class with a pre-defined configuration like this, than to use a base class with explicit arguments. In absolute terms, the same # of symbols are exposed to the user, but the semantics are simpler for PyomoPoolManager (i.e. they don't need to think about the as_solution option).
| as_solution : Function or None | ||
| Method for converting inputs into Solution objects. | ||
| A value of None will result in the default _as_solution function being used. |
There was a problem hiding this comment.
I like this callback design for its extensibility (and because in a lot of contexts we really only care about part of a solution, not every Var in the model, and this will allow for defining that as a "solution" for AOS purposes.). Can you document somewhere what the expected input and output for this function are? It seems like it is assumed it returns a Solution object, right? And it can actually be called given anything because it will pass through what is given to add?
There was a problem hiding this comment.
I agree that this needs better documentation.
I've renamed _as_solution to default_as_solution, and I've added documentation there.
I've also simplified the API, so that this function only is responsible for constructing and returning a solution object. I moved the checks for the special case where a Solution is passed-in to the add() method.
pyomo/contrib/alternative_solutions/tests/test_gurobi_lp_enum.py
Outdated
Show resolved
Hide resolved
|
@emma58 Thanks for all the feedback! I've gone through some of your comments, but I'll get back to this sometime next week. |
5 participants
Fixes #3513 (partially) .
Summary/Motivation:
The alternative_solutions contrib package includes a rudimentary solution pool object. This PR includes a more comprehensive capability for defining and managing solution pools.
Changes proposed in this PR:
NOTE: This is a WIP PR. I'm submitting this now to help define expectations for finalizing this new capability.
Legal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: